Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically closing tags in multi-cursor mode only closes one tag #490

Merged

Conversation

angelozerr
Copy link
Contributor

Automatically closing tags in multi-cursor mode only closes one tag

Fixes #225

Signed-off-by: azerr [email protected]

@angelozerr
Copy link
Contributor Author

The PR was easy to do, I have just copied/pasted code from https://github.com/microsoft/vscode/blob/main/extensions/html-language-features/client/src/tagClosing.ts

The only thing that I noticed is that the tagComplete coming from language server should return only string instead of string+range.

I will create a PR on LemMinx side to fix that, but you can play with the PR now, it should work.

@datho7561 datho7561 self-requested a review June 1, 2021 14:52
@datho7561
Copy link
Contributor

I decided to test this use case:

multi-cursor-close

We could try to support adding the correct close tag by passing an array of cursors in the xml/closeTag request. Another way might be to make multiple requests to LemMinX (i.e. request close for first cursor, apply the result, request close for second cursor, apply...)

Then again, it might not be worthwhile to support this use case, since you expect the same text to appear at all the cursors when using multi cursor mode.

@angelozerr
Copy link
Contributor Author

@datho7561 for the moment I would like to have the same behavior than HTML language feature. Your idea seems great, but perhaps we should discuss about that with vscode html language service team to have the same code tagClosing.ts

@fbricon
Copy link
Collaborator

fbricon commented Jun 2, 2021

@datho7561 nice catch! However I think we can fix it later, as you said, the common use case, from a user standpoint, is to multi-close the same tags.

Unrelated: to review the PR while ignoring whitespace changes, append ?w=1 to the files url: https://github.com/redhat-developer/vscode-xml/pull/490/files?w=1

@angelozerr angelozerr marked this pull request as ready for review June 2, 2021 14:44
@angelozerr
Copy link
Contributor Author

Unrelated: to review the PR while ignoring whitespace changes, append ?w=1 to the files url: https://github.com/redhat-developer/vscode-xml/pull/490/files?w=1

The code is just a copy/paste from html language feature of vscode.

@angelozerr angelozerr force-pushed the autoclose-multicursor branch 2 times, most recently from 5da7584 to 29a1cf7 Compare June 4, 2021 15:59
@datho7561 datho7561 self-requested a review June 4, 2021 16:03
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and works great! Just one small change

src/client/tagClosing.ts Outdated Show resolved Hide resolved
@angelozerr angelozerr force-pushed the autoclose-multicursor branch from 29a1cf7 to 19d0a62 Compare June 4, 2021 16:46
@datho7561 datho7561 merged commit 5f65a46 into redhat-developer:master Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically closing tags in multi-cursor mode only closes one tag
3 participants